fix(pip): do not use experimental_index_url for publish_deps #3311
Conversation
This usage means that all users will query `PyPI` to get SimpleAPI metadata. Once bazelbuild/bazel#24777 is merged, we can roll it back, but then we may need to check-in the `MODULE.bazel.lock` file into git. This should be no-op for most users because the publisher is usually run from with `host == exec`, but there is a small chance that people will hit bazel-contrib#2241. Work towards bazel-contrib#2937
Summary of ChangesHello @aignas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where publishing dependencies were configured to use Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly disables the use of experimental_index_url for publish_deps to work around a Bazel issue, as described in #2937. The changes in MODULE.bazel and CHANGELOG.md are accurate. For improved long-term maintainability, I've suggested adding a comment in MODULE.bazel to document the reason for this temporary change and the conditions under which it can be reverted.
|
Running the publisher as a build action doesn't make sense -- a build action isn't appropriate for e.g. uploading release artifacts. So I don't think we have to worry about that. |
|
I modified the PR description to better describe things ("This" wasn't clear if it was referring to before or after state). Approved; but check the PR description to make sure I understood correctly |
Currently, experimental_index_url will trigger network access (by the bzlmod extension
logic) as it figures out the wheels. This is problematic because this network
access always occurs, even if a user isn't using the publish tool. Additionally,
failed network accesses can get Bazel into stuck state requiring
bazel shutdownto fix it.
To fix, don't use experimental_index_url for the publish tool deps.
Once bazelbuild/bazel#24777 is merged, we can roll it back,
but then we may need to check-in the
MODULE.bazel.lockfile into git.This should be no-op for most users because the publisher is usually run
from with
host == exec, but there is a small chance that people willhit #2241.
Work towards #2937